Skip to content

More details php version information in diagnose #3609

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Nov 13, 2024

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Nov 6, 2024

as requested in #3584 (comment)

grafik

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shows me we need to do a bit of a refactoring.

Current state: ComposerPhpVersionFactory either reads phpVersion from phpstan.neon (if it has min+max), or from composer.json. And ConstantResolver uses ComposerPhpVersionFactory for this logic.

Wanted state: ComposerPhpVersionFactory only reads PHP version from composer.json, as its name says. And then ConstantResolver decides if it should read phpVersion from phpstan.neon with min/max provided. And if it's not there then it should read it from ComposerPhpVersionFactory.

Feel free to do this in a separate PR before this one. Thanks.

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say the composer.json PHP version is irrelevant if there's phpVersion.min+max version in phpstan.neon. The diagnose command should reflect that and print composer.json version only if phpVersion in phpstan.neon is int or null.

@ondrejmirtes
Copy link
Member

Looking at ComposerPhpVersionFactory usages, I found a bug: VersionCompareFunctionDynamicReturnTypeExtension only looks at composer version, not phpVersion.min+max version.

@staabm
Copy link
Contributor Author

staabm commented Nov 13, 2024

Looking at ComposerPhpVersionFactory usages, I found a bug: VersionCompareFunctionDynamicReturnTypeExtension only looks at composer version, not phpVersion.min+max version.

I will send a separate PR

@ondrejmirtes
Copy link
Member

Awesome, now it makes sense to me :)

@ondrejmirtes ondrejmirtes merged commit f243636 into phpstan:2.0.x Nov 13, 2024
422 checks passed
@staabm staabm deleted the diag branch November 13, 2024 13:00
@@ -2108,6 +2108,7 @@ services:
arguments:
composerAutoloaderProjectPaths: %composerAutoloaderProjectPaths%
allConfigFiles: %allConfigFiles%
configPhpVersion: %phpVersion%
autowired: false

# Error formatters

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix_Resolve_Complete

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants